feat: add an option to preserve whitespace to FullSanitizer#157
feat: add an option to preserve whitespace to FullSanitizer#157Earlopain wants to merge 1 commit intorails:mainfrom
Conversation
|
Hey, @Earlopain, sorry for my slow reply. Give me a day to come up for air and I'll be able to reply thoughtfully. |
Earlopain
force-pushed
the
full-sanitizer-preserve-whitespace
branch
from
c8afae5 to
48d2142
Compare
|
It's all good, no worries. I admit I am a bit lost with the recent changes to main, I'm not sure how I should integrate the changes now. |
Earlopain
force-pushed
the
full-sanitizer-preserve-whitespace
branch
from
48d2142 to
82fea0e
Compare
Earlopain
force-pushed
the
full-sanitizer-preserve-whitespace
branch
from
82fea0e to
9a336ad
Compare
|
After some deliberation I have something that at least works. Sorry about the notifications for force-pushing a bunch. It should now be in about the same state it was before, just pointing at current main. I'm not a big fan how I did it with the module but that was the first and only thing that came to my mind. Feels like something I would monkeypatch when I don't control the code myself. I do like how the tests are structured now, nice job on that. I removed the html4 example from the readme, since just one section above it mentions that the examples work both with html4 and html5. That way I only need to add the new argument to one example. There also appears to be stray |
|
Note to casual readers: a conversation is ongoing about this topic at the original issue #154 |
| ``` | ||
|
|
||
| or, if you insist on parsing the content as HTML4: | ||
| # Whitespace is swallowed by default. If whitespace is significant you must pass an option to preserve it. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
3 participants
Closes #154
There are still a few things missing. @flavorjones perhaps you want to give some feedback in advance?
How thorough should I be for these tests? Ideally Loofah already has good coverage, I don't want to duplicate that unnecessarily. I do however assert that escaping is identical for both methods which I thought important. I added a test helper for that and tweaked a small amount of tests so that there are no output differences because of whitespace.
Docs are missing, will get to those.